[RFC] Add BOLT 12 payer proof primitives#4297
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4297 +/- ##
===========================================
+ Coverage 28.02% 86.81% +58.79%
===========================================
Files 126 160 +34
Lines 69960 112368 +42408
Branches 69960 112368 +42408
===========================================
+ Hits 19606 97555 +77949
+ Misses 49020 12249 -36771
- Partials 1334 2564 +1230
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
left a comment
There was a problem hiding this comment.
A few notes, though I didn't dig into the code at a particularly low level.
2324361 to
9f84e19
Compare
Add a Rust CLI tool that generates and verifies test vectors for BOLT 12 payer proofs as specified in lightning/bolts#1295. The tool uses the rust-lightning implementation from lightningdevkit/rust-lightning#4297. Features: - Generate deterministic test vectors with configurable seed - Verify test vectors from JSON files - Support for basic proofs, proofs with notes, and invalid test cases - Uses refund flow for explicit payer key control Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🔔 1st Reminder Hey @valentinewallace! This PR has been waiting for your review. |
TheBlueMatt
left a comment
There was a problem hiding this comment.
Some API comments. I'll review the actual code somewhat later (are we locked on on the spec or is it still in flux at all?), but would be nice to reduce allocations in it first anyway.
|
🔔 2nd Reminder Hey @valentinewallace! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @jkczyz! This PR has been waiting for your review. |
fb8c68c to
9ad5c35
Compare
Add a `payer_proof_deser` fuzz target that runs arbitrary bytes through `PayerProof::try_from`, which reconstructs the invoice merkle root from the selective-disclosure data (leaf/missing hashes and omitted markers), runs the omitted-marker validation, the invoice and payer signature checks, and the TLV deserialization. On a successful parse it asserts the parsed bytes round-trip, matching the other BOLT 12 `*_deser` targets. Wired up the same way as the other BOLT 12 `*_deser` targets: a `do_test` in fuzz/src, a module in fuzz/src/lib.rs, and a `GEN_FAKE_HASHES_TEST` entry regenerating fuzz/targets.h and the fake-hashes bin target. Requested in review on lightningdevkit#4297. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TheBlueMatt
left a comment
There was a problem hiding this comment.
Needs rebase, sadly.
| MerkleError(SelectiveDisclosureError), | ||
| /// The invoice signature is invalid. | ||
| InvalidInvoiceSignature, | ||
| /// Failed to re-derive the payer signing key from the provided nonce and payment ID. | ||
| KeyDerivationFailed, | ||
| /// The given TLV type cannot be included in a payer proof. Carries the offending | ||
| /// type number. Reasons include `PAYER_METADATA_TYPE`, TLVs in `SIGNATURE_TYPES`, | ||
| /// or TLVs in `PAYER_PROOF_DATA_TYPES`. | ||
| DisallowedTlvType(u64), | ||
|
|
||
| /// Error decoding the payer proof. | ||
| DecodeError(DecodeError), |
There was a problem hiding this comment.
nit: in general I'm not a big fan of super granular error enums as a way to communicate developer-relevant data. In general, if an error requires different programatic handling by the caller it should have its own enum variant, if the handling is just the same "just log/print the error and consider it failed" then it doesn't need an enum variant. Same goes double for SelectiveDisclosureError - do we need it at all or would a &'static str suffice?
InvoiceRequest and Refund have payer metadata consisting of an encrypted payment id and, originally, a nonce used to derive the payer signing keys and authenticate any corresponding invoices. The nonce was elided to save space once it was included in the OffersContext of blinded reply paths, but that means verifying a Bolt12Invoice requires state outside the invoice itself. Upcoming payment proofs (lightningdevkit#4297) need the invoice signing keys derivable from the invoice request alone, so include the nonce in the payer metadata again and verify invoices using it rather than the context's nonce. Co-Authored-By: Claude <noreply@anthropic.com>
InvoiceRequest and Refund have payer metadata consisting of an encrypted payment id and, originally, a nonce used to derive the payer signing keys and authenticate any corresponding invoices. The nonce was elided to save space once it was included in the OffersContext of blinded reply paths, but that means verifying a Bolt12Invoice requires state outside the invoice itself. Upcoming payment proofs (lightningdevkit#4297) need the invoice signing keys derivable from the invoice request alone, so include the nonce in the payer metadata again and verify invoices using it rather than the context's nonce. This breaks verification of invoices for invoice requests and refunds with blinded paths created by prior versions, as their payer metadata lacks the nonce; such payments will fail and must be retried with a new payment id. Refunds without blinded paths are unaffected, as their metadata always included the nonce. Co-Authored-By: Claude <noreply@anthropic.com>
93f4104 to
73eea68
Compare
8d543ce to
5aedb03
Compare
Move the invoice/refund payer key derivation logic into reusable helpers so payer proofs can derive the same signing keys without duplicating the metadata and signer flow.
Extend the BOLT 12 merkle module with selective-disclosure support: build the full merkle tree from a TLV stream, compute the omitted-TLV markers and the minimal set of missing hashes for omitted subtrees, and reconstruct the merkle root from a partial disclosure. These are the primitives a payer proof is built on. Co-Authored-By: Rusty Russell <rusty@rustcorp.com.au> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: OpenAI Codex <codex@openai.com>
Add the `payer_proof` module: `PayerProof`/`UnsignedPayerProof`, the `PayerProofBuilder` (with selective disclosure and a derived-key path), bech32 `lnp` encoding, and parse-time verification, implementing the payer proof extension to BOLT 12 (lightning/bolts#1295). Also exposes the offer/invoice TLV-type constants and an invoice-bytes accessor used to build proofs, and a `Sha256` `Writeable`/`Readable` impl for the proof hashes. Co-Authored-By: Rusty Russell <rusty@rustcorp.com.au> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: OpenAI Codex <codex@openai.com>
Carry the paid `Bolt12Invoice` through the outbound payment so it survives restarts, and surface it as a `PaidBolt12Invoice` on `Event::PaymentSent` so the payer can build a payer proof. The payer signing key is re-derived from the invoice's own payer metadata, so no extra key material is stored. `PaidBolt12Invoice` now lives in `offers::payer_proof`; existing async payment tests and a test helper are updated to construct it via the new API. Adds an end-to-end test that pays a BOLT 12 offer and builds + verifies a payer proof from the resulting `Event::PaymentSent`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Throw arbitrary bytes at `PayerProof::try_from` to exercise the merkle-root reconstruction and the deserialization path together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: OpenAI Codex <codex@openai.com>
| while inc_idx < included_types.len() || mrk_idx < omitted_markers.len() { | ||
| if mrk_idx >= omitted_markers.len() { | ||
| // No more markers, remaining positions are included | ||
| positions.push(true); | ||
| inc_idx += 1; | ||
| } else if inc_idx >= included_types.len() { | ||
| // No more included types, remaining positions are omitted | ||
| positions.push(false); | ||
| prev_marker = omitted_markers[mrk_idx]; | ||
| mrk_idx += 1; | ||
| } else { | ||
| let marker = omitted_markers[mrk_idx]; | ||
| let inc_type = included_types[inc_idx]; | ||
|
|
||
| if marker == next_marker(prev_marker) { | ||
| // Continuation of current run → this position is omitted | ||
| positions.push(false); | ||
| prev_marker = marker; | ||
| mrk_idx += 1; | ||
| } else { | ||
| // Jump detected! An included TLV comes before this marker. | ||
| // After the included type, prev_marker resets to that type, | ||
| // so the marker will be processed as a continuation next iteration. | ||
| positions.push(true); | ||
| prev_marker = inc_type; | ||
| inc_idx += 1; | ||
| // Don't advance mrk_idx - same marker will be continuation next | ||
| } | ||
| } |
There was a problem hiding this comment.
Can't this drift from what's essentially the same loop in reconstruct_merkle_root? We should DRY this up.
There was a problem hiding this comment.
What do you think about this solution? 564d463
|
|
||
| #[inline] | ||
| pub fn do_test<Out: test_logger::Output>(data: &[u8], _out: Out) { | ||
| if let Ok(payer_proof) = PayerProof::try_from(data.to_vec()) { |
There was a problem hiding this comment.
We should also update the invoice fuzzer to build a payer proof, just like how the offer fuzzer builds an invoice request and how the invoice request fuzzer builds an invoice.
There was a problem hiding this comment.
The preimage gate makes this a no-op if I put it in invoice_deser. Building a proof requires SHA256(preimage) == invoice.payment_hash(), and invoice_deser parses arbitrary invoices, so we never hold a matching preimage. prove_payer() returns PreimageMismatch before any of the interesting code runs (build_unsigned doesn't even re-check the invoice signature, the preimage is the only gate), so we'd just be exercising the early error return.
Claude help me find the place where we actually control the payment hash is invoice_request_deser, since it builds the invoice itself via respond_with(paths, payment_hash). If I set payment_hash = SHA256(known_preimage) there, I can chain a full payer proof build+sign right after the invoice is signed, which exercises the selective disclosure, merkle tree, and proof signing for real.
Want me to add it in invoice_request_deser that way? Or do you still want the call in invoice_deser even though it only ever hits PreimageMismatch?
This is a first draft implementation of the payer proof extension to BOLT 12 as proposed in lightning/bolts#1295. The goal is to get early feedback on the API design before the spec is finalized.
Payer proofs allow proving that a BOLT 12 invoice was paid by demonstrating possession of:
This PR adds the core building blocks:
This is explicitly a PoC to validate the API surface - the spec itself is still being refined. Looking for feedback on:
cc @TheBlueMatt @jkczyz